@ikelos - I'd love your thoughts on this. The idea to have a simple to use way to get key information about a process for plugins. That way there can be a central way to do these lookups within plugins so that they're consistent and if an update needs to be done it only needs to happen in one place. It could be bulked out to include extra bits for a process like getting memory regions etc.

It should hopefully mean that we'd be able to make non-OS specific plugins - assuming it's done in the right way. Given it's above intel process that might be a good place for it given the arm work at the moment?

Thanks again.

Yep, sorry, it is on my list, just a bit lower down at the moment, sorry! 5:S

Hey @eve-mem ! It's a great idea to ensure better consistency across the framework.

I think this is also related to your other ticket Unify Linux plugins representation of pids which we should take a decision at some point.

Thanks both. I completely understand not having time to look at this given how little time i seem to have at the moment.

Thanks for those pointers @gcmoreira - that's almost exactly why I'd pike something like this PR to exist. Just a single place to have the 'right' answer so we can do it once for all plugins. Not necessarily this version in particular, but the idea at least.

Hey @ikelos and @gcmoreira,

I was working on the PR for #981 and it just felt to me that doing this first might make it all a bit easier.

Assuming that you both think that this is a good idea of course! What do you think?

I also tried to add an explanation of tgid/pid/tid into the readme docs. Not sure what you both make of that?

@gcmoreira thanks for the comments - I think I've got them all now. I'm a little unsure I got the datetime part correct in the mac side - could you check that a little closer?

Thanks both.
🦊

Hey @eve-mem good job!
Yeah the aware datetime change is correct.

I think it's a solid idea, but we should keep refining the implementation. There are a few areas that could be improved, and some issues that still need fixing.

• GenericProcess:
  • Should include methods to return all TGID and PID (user PID and TID) and their respective parents. Not only the Thread Group Leader, otherwise we would be burying the threads behind that interface.
  • I don't think the method names do much to clear up the confusion between the kernel PID/TGID/TID/etc. In fact, it feels like we're just shifting the problem from one class to another. Even the docstrings are not helping at all to that purpose. We should be more explicit with the method names. We should choose a point of view and stick to it throughout the framework. If we decide to take the user-space's perspective, I think it should be better to use names like get_user_pid(), get_user_tid(), and so on. For instance, see below that with the current implementation (in red) even with the comment clarifying it, it's still confusing. This can be much clear if we use more explicit method names and include the user TID (green changes).

-    user_pid = task.get_pid()
-    user_tid = task.pid  # Note that pid is user_tid in Linux
-    user_ppid = task.get_parent_pid()
+    user_pid = task.get_user_pid()
+    user_tid = task.get_user_tid()
+    user_ppid = task.get_user_ppid() 
# or get_parent_user_pid()

• Filter: I strongly believe it's a mistake to filter by user_pid (TGID). Actually, with the proposed implementation we are changing the way Volatility3 is working now, since we are currently filtering by user_tid (PID). If we change to use user_pid (TGID) that makes impossible to filter by TID. Also, note that this affects not just the pslist plugin --filter argument, which can now be superseded by the more general --filters argument and maybe removed, but with the changes in create_pid_filter() here this is affecting other current and future plugins which may require to get a specific thread and not the thread group leader. With this change we are introducing a bug and no one in the framework will be able to find a specific thread. I explained this with some examples here. I believe it's quite clear, but if it's not, please let me know, and I'll do my best to clarify that explanation.
  • On the contrary, even for users unfamiliar with the meaning of "a pid" in the framework, using user_tid (PID) will yield the expected result, since user_pid == user_tid (or in kernel space task_struct.tgid == task_struct.pid) for a thread group leader (what is typically referred to as 'a process' in user-space).
• We should also extend these changes to the create_pid_filter() method name to stick to the proposed point of view. With the Volatility3's current behavior (filtering by user_tid) it should be renamed to create_user_tid_filter()
• Review all occurrences of the term pid in the framework and adjust accordingly to align with the proposed perspective.
• Lastly, we should avoid using the abbreviation upid for user_pid, as it has a specific meaning in the Linux kernel see here. I'm not saying you are already doing this, I just want to flag this in case we feel tempted to use it. I might be the first to slip up on my own words LOL! That's why peer reviews are crucial.

Thanks @gcmoreira - makes sense I'll work on it. Particularly the pid filtering, it makes sense to me now.

I was hoping that the functions would be the same across the different OSes, I'll change the get_pid to get_user_pid for all of them.